Repository navigation
fix(build): build MLX without null-this traps (needed for MLX >= 0.32.1) - #2384
Open
AlexCheema wants to merge 1 commit into
Open
AlexCheema wants to merge 1 commit into
AlexCheema wants to merge 1 commit into
Conversation
metal-cpp calls Objective-C methods through pointers that may be nil and relies on messages to nil being no-ops: SharedPtr's destructor calls m_pObject->release() on a moved-from null. In C++ a member call through a null pointer is undefined, and clang 21 (the nixpkgs compiler that builds MLX here) uses that: where it can see the pointer is null, it compiles the path to brk #1. Apple's clang 17, which builds MLX's official wheels, doesn't. With MLX >= 0.32.1, this broke loading any model that needs a fourth Metal residency set (over ~12 GB on a 96 GB Mac): vector::push_back's fast path in ResidencySets::add_set_locked became a trap and the runner died with SIGTRAP. The official 0.32.3 wheel loads the same model. Building with -fno-delete-null-pointer-checks keeps the Objective-C semantics metal-cpp needs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Nix build of MLX (on macOS) uses nixpkgs' clang 21. metal-cpp, which MLX uses for Metal, calls Objective-C methods through pointers that may be nil, and relies on Objective-C's rule that a message to nil does nothing. For example,
NS::SharedPtr's destructor callsm_pObject->release()on a moved-from null. In C++, a member call through a null pointer is undefined behaviour. Where clang 21 can see that the pointer is null, it compiles that path tobrk #1. Apple's clang 17, which builds MLX's official wheels, doesn't.This bites as soon as exo moves past MLX 0.32.0. MLX 0.32.1 (ml-explore/mlx#4211) spreads a model's memory over several Metal residency sets. In
ResidencySets::add_set_locked, the fast path ofsets_.push_back(Set{std::move(set), 0})became a trap. The first three sets go through the reallocating path; the fourth finds spare capacity and traps. So loading any model that needs a fourth set, over ~12 GB on a 96 GB Mac, killed the runner with SIGTRAP.Measured on a Mac Studio (M3 Ultra, 96 GB) with MLX main (
0e3ff364), which has the fast-synch deadlock fixes ml-explore/mlx#4552 and #4556:-fno-delete-null-pointer-checksReproduction: a 12-line file, a
std::vectorof a struct holding anNS::SharedPtrwithpush_back(Set{std::move(p), 0}). nixpkgs clang 21 at-O3emitsbrk #1; Xcode's clang 17 doesn't, and nor does clang 21 with the flag.Change
Build MLX with
-fno-delete-null-pointer-checks(CMAKE_CXX_FLAGSinpython/parts.nix), which keeps the semantics metal-cpp depends on.With the MLX fork exo pins today (based on 0.32.0) this is latent. No runner died with SIGTRAP in any of our chaos runs on it. But any MLX upgrade needs it, and the null-pointer pattern is used throughout metal-cpp.
Tests
ResidencySets::add_set_lockedbefore and after: the in-place construction is back instead ofbrk #1.🤖 Generated with Claude Code